Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Avoid type-checking addition and indexing twice. #40863

Merged
merged 1 commit into from
Apr 6, 2017
Merged

Avoid type-checking addition and indexing twice. #40863

merged 1 commit into from
Apr 6, 2017

Conversation

eddyb
Copy link
Member

@eddyb eddyb commented Mar 27, 2017

Fixes #40610 by moving the common check_expr_coercable_to_type call before the error reporting logic for binops and removing the one from check_str_addition.
Fixes #40861 by removing an unnecessary check_expr_coercable_to_type call.

@eddyb eddyb added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Mar 27, 2017
@rust-highfive
Copy link
Collaborator

r? @pnkfelix

(rust_highfive has picked a reviewer for you, use r? to override)

@arielb1
Copy link
Contributor

arielb1 commented Mar 27, 2017

@bors r+

@bors
Copy link
Contributor

bors commented Mar 27, 2017

📌 Commit 112f36a has been approved by arielb1

@eddyb
Copy link
Member Author

eddyb commented Mar 27, 2017

@bors r- Travis failed (I'll investigate later)

@nikomatsakis nikomatsakis added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Apr 4, 2017
@nikomatsakis
Copy link
Contributor

Accepting for beta, once @eddyb fixes travis failures.

Small patch, regression.

cc @rust-lang/compiler

@alexcrichton alexcrichton mentioned this pull request Apr 4, 2017
Merged
bors added a commit that referenced this pull request Apr 5, 2017
@bors
Auto merge of #41071 - alexcrichton:beta-next, r=alexcrichton
ca3d118 
[beta] Backport accepted nominations

This is a backport of

* #40813
* #40849

This also includes #41069

Finally, this includes a bump to beta .3.

This is all current nominations except #40863, which is not passing tests yet.
@eddyb eddyb force-pushed the coerce-only-once branch from 112f36a to edc7f9a Compare April 6, 2017 18:42
@eddyb
Copy link
Member Author

eddyb commented Apr 6, 2017

I suspect there's another bug elsewhere but at least I hope I can make this work.

eddyb
eddyb commented Apr 6, 2017
View reviewed changes
src/librustc_typeck/check/op.rs
@@ -257,9 +262,6 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
}
};

// see `NB` above
self.check_expr_coercable_to_type(rhs_expr, rhs_ty_var);

(rhs_ty_var, return_ty)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests weren't passing locally (actually, libcollections wasn't building at all after a rebase) when rhs_ty_var was replaced with rhs_ty.

@eddyb
Copy link
Member Author

eddyb commented Apr 6, 2017

@bors r=arielb1

@bors
Copy link
Contributor

bors commented Apr 6, 2017

📌 Commit edc7f9a has been approved by arielb1

@eddyb
Copy link
Member Author

eddyb commented Apr 6, 2017

cc @alexcrichton This should be ready for backport now, apologies for the delay.

@bors
Copy link
Contributor

bors commented Apr 6, 2017

⌛ Testing commit edc7f9a with merge 50c1864...

bors added a commit that referenced this pull request Apr 6, 2017
@bors
Auto merge of #40863 - eddyb:coerce-only-once, r=arielb1
50c1864 
Avoid type-checking addition and indexing twice.

Fixes #40610 by moving the common `check_expr_coercable_to_type` call before the error reporting logic for binops and removing the one from `check_str_addition`.
Fixes #40861 by removing an unnecessary `check_expr_coercable_to_type` call.
@bors
Copy link
Contributor

bors commented Apr 6, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: arielb1
Pushing 50c1864 to master...

@bors bors merged commit edc7f9a into rust-lang:master Apr 6, 2017
@eddyb eddyb deleted the coerce-only-once branch April 7, 2017 06:22
@alexcrichton alexcrichton mentioned this pull request Apr 20, 2017
Merged
@alexcrichton alexcrichton removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Apr 20, 2017
bors added a commit that referenced this pull request Apr 21, 2017
@bors
Auto merge of #41427 - alexcrichton:beta-next, r=alexcrichton
3cc1e39 
[beta] Final backports to beta

Backport of:

* #40863
* #41085
* #41354
* #41378
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@pnkfelix pnkfelix

Labels
beta-accepted Accepted for backporting to the compiler in the beta channel.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@eddyb @rust-highfive @arielb1 @bors @nikomatsakis @alexcrichton @pnkfelix